Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add x-model support if variable is not defined in x-data #3798

Closed
wants to merge 9 commits into from
Closed

Add x-model support if variable is not defined in x-data #3798

wants to merge 9 commits into from

Conversation

yanaleksandrov
Copy link

@yanaleksandrov yanaleksandrov commented Oct 6, 2023

When using the x-model directive, if no variable is declared in x-data, this leads to an error Uncaught ReferenceError: variable is not defined. Therefore, the variable must be declared in the x-data directive.

Problem
The complication of the html document.

Example
In one project we have a form containing more than a hundred fields. The HTML form looks like this:

<form x-data="{
test: '',
years: '',
cb: '',
colors: '',
radio: '',
textarea: '',
color: '',
colorList: '',
dynamicallyColors: '',
.... and many more variables
}">
    // form content
</form>

If we want to fill the form with data, the html gets clogged up even more, the form takes on the appearance of:

<form x-data="{test: 'Test content', years: 56, cb: true, colors: ['red','green'], radio: 'rb1', textarea: 'My message with 10000 symbols', color: 'white', colorList: '', dynamicallyColors: '', .... and many more variables}">
    // form content
</form>

However, in this implementation we have a problem with waiting. While JS finishes its work and the data is parse, the form "blinks". To solve this problem, we fill the form with data in the fields themselves. All this together bloats the HTML code and it becomes difficult to read.

The problem takes on magnitude when we are using huge data. For example, we repeat the text from the textarea in both the x-data="{textarea: 'OH NO! very long text'}" directive and in the <textarea x-model="textarea">OH NO! very long text</textarea> tag itself. This is contrary to the DRY principle.

Solution
I suggest adding an auto modifier that will automatically parse the parent x-data of the x-model element, and if it is not declared, will be added when x-model.auto is initialized. If the variable is not declared, x-model will try to get the value from the input, select or textarea element itself.

HTML form will look neater. For example for testing:

<div x-data>
    <div>
        Text
        <input type="text" x-model.auto="test" placeholder="Enter text">
        <span x-text="test"></span>
    </div>
    <br>
    <div>
        Number
        <input type="number" x-model.auto="years" placeholder="Enter number" value="Text content">
        <span x-text="years"></span>
    </div>
    <br>
    <div>
        Single checkbox with boolean
        <input type="checkbox" value="red" x-model.auto="cb">
        <span x-text="cb"></span>
    </div>
    <br>
    <div>
        Multiple checkboxes bound to array
        <input type="checkbox" value="red" x-model.auto.lazy="colors">
        <input type="checkbox" value="orange" x-model.auto.lazy="colors">
        <input type="checkbox" value="yellow" x-model.auto.lazy="colors">
        <span x-text="colors"></span>
    </div>
    <br>
    <div>
        Radio Button
        <input type="radio" value="rb1" x-model.auto="radio"> Radio Button 1
        <input type="radio" value="rb2" x-model.auto="radio"> Radio Button 2
        <span x-text="radio"></span>
    </div>
    <br>
    <div>
        Textarea
        <textarea x-model.auto="textarea"></textarea>
        <span x-text="textarea"></span>
    </div>
    <br>
    <div>
        Select
        <select x-model.auto="color">
            <option value="">Choose color</option>
            <option value="red">Red</option>
            <option value="orange">Orange</option>
            <option value="yellow">Yellow</option>
        </select>
        Color: <span x-text="color"></span>
    </div>
    <br>
    <div>
        Multiple Select
        <select x-model.auto="colorList" multiple>
            <option>Red</option>
            <option>Orange</option>
            <option>Yellow</option>
        </select>
        Color: <span x-text="colorList"></span>
    </div>
    <br>
    <div>
        Dynamically populated Select Options
        <select x-model.auto="dynamicallyColors">
            <template x-for="color in ['Red', 'Orange', 'Yellow']">
                <option :value="color" x-text="color"></option>
            </template>
        </select>
        Color: <span x-text="dynamicallyColors"></span>
    </div>
</div>

I invite discussion on this realization. Thank you.

@ekwoka
Copy link
Contributor

ekwoka commented Oct 7, 2023

Can you add a test for this?

@yanaleksandrov
Copy link
Author

yanaleksandrov commented Oct 7, 2023

Of course, I updated the PR

On the other hand, I think instead of producing new modifiers, it is better to add this feature to the basic implementation of the x-model. But it's better to discuss it.

@ekwoka
Copy link
Contributor

ekwoka commented Oct 7, 2023

btw, in the meantime, you can just put all the props in an object instead.

like on the x-data add a fields: {}

then x-model.fill="fields.cb"

This will basically work as expected or close to it

I think part of this being attached to the fill modifier can make sense.

There is already a pattern for how setting undefined things behaves, so tapping into that should work.

@yanaleksandrov
Copy link
Author

@ekwoka Thanks for the idea. Exactly, the functionality is very similar, and this partially solves the problem. And the meaning of x-model.auto essentially disappears. On the other hand, I think it would be great to make the behavior of "x-model" as "x-model.fill" by default.

@ekwoka
Copy link
Contributor

ekwoka commented Oct 7, 2023

That could be argued is a breaking change.

@yanaleksandrov
Copy link
Author

@ekwoka I test x-model with fill modificator, and, unfortunately, input still does not take values from the value attribute.
Therefore, the problem with data duplication is remains ¯ \ _ (ツ) _ / ¯

<form x-data="{fields: {}}">
	<input value="Test text" x-model.fill="fields.test">
	<span x-text="fields.test"></span>
</form>

@yanaleksandrov
Copy link
Author

yanaleksandrov commented Oct 7, 2023

@ekwoka Updated PR. I suggest then, instead of adding the new auto modifier, simply expand the fill modifier. Now instead of:

<form x-data="{
    test: '',
    years: '',
    cb: '',
    colors: '',
    radio: '',
    textarea: '',
    color: '',
    colorList: '',
    dynamicallyColors: '',
    .... and many more variables
}">
    // list of input fields
</form>

Can write shorter:

<form x-data>
    // list of input fields
</form>

let value = '';
if (el.type === 'checkbox' || el.type === 'radio') {
const attribute = `x-model\\.${modifiers.join('\\.')}`,
items = parent.querySelectorAll(`[${attribute}="${expression}"]`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure none of the stuff here needs to be done at all. Each element with x-model will initialize and do their own checks. You don't need to try to find them.

if (typeof expression === 'string') {
let parent = el.closest('[x-data]');
if (parent) {
let xData = Alpine.$data(parent);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just call $data on the element directly. You don't need to find the root yourself.

let parent = el.closest('[x-data]');
if (parent) {
let xData = Alpine.$data(parent);
if (typeof xData[expression] === 'undefined') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will incorrectly trigger this branch if the model is an expression and not a literal key (ie. 'info.query')

@ekwoka
Copy link
Contributor

ekwoka commented Oct 8, 2023

Some follow up context, what is the reason you need all the info put into Alpine and not just use native form behaviors?

In your example, just making a root a form element you can get all the data with just new FormData($root) without any modeling behaviors.

@yanaleksandrov
Copy link
Author

Some follow up context, what is the reason you need all the info put into Alpine and not just use native form behaviors?

In your example, just making a root a form element you can get all the data with just new FormData($root) without any modeling behaviors.

There are conditional logic in our form, and we would like to hide/show form elements by values of other fields.

@yanaleksandrov
Copy link
Author

@ekwoka PR is updated

@yanaleksandrov
Copy link
Author

However, I do not even know if it is possible to do something, if the property is called before its initialization in the "x-model", the problem with the undefined property remains :( Оnly if declare the property in x-data, but then there is no sense in this PR.

E.g.:

<div x-data>
    <span x-text="test"></span>
    <input type="text" x-model.fill="test" placeholder="Enter text" value="String text input">
</div>

@ekwoka
Copy link
Contributor

ekwoka commented Oct 12, 2023

true, or at least it would keep looking up the stack for it, the non the global.

You could make a fancy proxy object to store the data in that just always returns null, allowing x-model.fill to work without changing any internals of alpine.

I was more on the side that model.fill should work on undefined, just allowing a plain object to be used like that.

In cases where we've needed behavior like you describe, we just have the few we care about defined on the x-data (with the initial values as well) to toggle them, or have the toggle happen with a request to the server to change the form element out. depending on the concerned behavior, etc.

@yanaleksandrov yanaleksandrov closed this by deleting the head repository Oct 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants